Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughIntroduces like/follow mutations with optimistic updates and wiring across story cards and pages; adds club creation/search flows with image upload and join mutation; new member/profile endpoints and queries; adds no-scrollbar CSS and several new API endpoint/type definitions. (49 words) Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Client UI
participant Hook as Mutation Hook
participant Service as Service (story/member)
participant API as Backend API
participant QC as QueryClient / Cache
UI->>Hook: user clicks like / follow
Hook->>Service: call toggle endpoint (toggleLike/toggleFollow)
Service->>API: POST /like or POST /follow
API-->>Service: 200 OK (result)
Service-->>Hook: returns result
Hook->>QC: optimistic update onMutate (update lists/detail)
QC-->>UI: UI reflects optimistic state
Hook->>QC: onSettled -> invalidate relevant queries
QC-->>UI: UI syncs with server state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 다른 사용자의 프로필을 조회하고 상호작용하는 기능을 도입하며, 모임 생성 및 검색 기능을 실제 API와 연동하여 사용자 경험을 크게 개선합니다. 좋아요 및 팔로우 기능에 낙관적 업데이트를 적용하여 UI 반응성을 높였고, 전반적인 데이터 흐름을 실제 백엔드 서비스와 통합했습니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/base-ui/Group-Search/search_mybookclub.tsx (1)
75-85: 🛠️ Refactor suggestion | 🟠 MajorUse
<Link>instead of a clickable<div>for navigation.The current implementation uses a
<div>with anonClickhandler for navigation, which has accessibility issues:
- Not keyboard-accessible (no
tabIndex,onKeyDown, orrole)- Loses native link behaviors (right-click to open in new tab, etc.)
For navigation, prefer Next.js
<Link>component.♻️ Proposed fix using Link
First, add the import:
import { useRouter } from "next/navigation"; +import Link from "next/link";Then replace the clickable div:
- {displayGroups.map((group) => ( - <div - key={group.id} - onClick={() => router.push(`/groups/${group.id}`)} - className="flex w-full h-[36px] t:h-[52px] py-3 px-4 items-center rounded-lg bg-white hover:brightness-98 hover:-translate-y-[1px] cursor-pointer" - > - <span className="text-Gray-7 body_1_2 t:subhead_4_1"> - {group.name} - </span> - </div> - ))} + {displayGroups.map((group) => ( + <Link + key={group.id} + href={`/groups/${group.id}`} + className="flex w-full h-[36px] t:h-[52px] py-3 px-4 items-center rounded-lg bg-white hover:brightness-98 hover:-translate-y-[1px] cursor-pointer" + > + <span className="text-Gray-7 body_1_2 t:subhead_4_1"> + {group.name} + </span> + </Link> + ))}Note: If you switch to
<Link>, theuseRouterimport on line 5 and theroutervariable on line 27 can be removed if they're no longer used elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/base-ui/Group-Search/search_mybookclub.tsx` around lines 75 - 85, Replace the clickable div used in displayGroups.map (the element with key={group.id} and onClick={() => router.push(`/groups/${group.id}`)}) with Next.js Link navigation: import Link from 'next/link', use <Link href={`/groups/${group.id}`} ...> to preserve the existing className, children (the span showing group.name) and key, and remove the useRouter import and router variable if they are no longer used elsewhere; this restores native link behavior (keyboard, context menu, new-tab) and fixes the accessibility issues from using a div with onClick.src/app/(main)/page.tsx (1)
106-124:⚠️ Potential issue | 🟠 MajorHome story cards expose like state but don’t wire the like action.
likedByMeis passed, butonLikeClickis missing in all three card mappings, so like taps become no-ops.🛠️ Suggested fix
<BookStoryCardLarge key={story.bookStoryId} id={story.bookStoryId} @@ likedByMe={story.likedByMe} + onLikeClick={() => toggleLike(story.bookStoryId)} coverImgSrc={story.bookInfo.imgUrl} @@ <BookStoryCard key={story.bookStoryId} id={story.bookStoryId} @@ likedByMe={story.likedByMe} + onLikeClick={() => toggleLike(story.bookStoryId)} coverImgSrc={story.bookInfo.imgUrl} @@ <BookStoryCard key={story.bookStoryId} id={story.bookStoryId} @@ likedByMe={story.likedByMe} + onLikeClick={() => toggleLike(story.bookStoryId)} coverImgSrc={story.bookInfo.imgUrl}Also applies to: 162-180, 220-238
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(main)/page.tsx around lines 106 - 124, The BookStoryCardLarge instances are rendering the current likedByMe state but not wiring the like action, so clicks do nothing; add an onLikeClick prop to each BookStoryCardLarge that invokes the existing like handler (e.g., call toggleLike or the component's like handler used elsewhere) with the story identity and current liked state (for example pass story.bookStoryId and story.likedByMe or an object the handler expects). Update the mappings that create BookStoryCardLarge (the shown block and the other two similar mappings) to pass onLikeClick={() => toggleLike({ bookStoryId: story.bookStoryId, liked: story.likedByMe })} (or the equivalent signature your toggleLike accepts) so likes are dispatched when the user taps the like button.
🧹 Nitpick comments (3)
src/lib/api/endpoints/index.ts (1)
6-7: Inconsistent file naming convention.The new exports
"./Clubs"and"./Image"use PascalCase, while existing modules (base,auth,bookstory,member,book) use lowercase. Consider renaming for consistency.♻️ Suggested consistency fix
-export * from "./Clubs"; -export * from "./Image"; +export * from "./clubs"; +export * from "./image";This would require renaming the actual files
Clubs.ts→clubs.tsandImage.ts→image.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/api/endpoints/index.ts` around lines 6 - 7, The exports in index.ts use PascalCase ("./Clubs", "./Image") which is inconsistent with the existing lowercase module names; rename the module files Clubs.ts → clubs.ts and Image.ts → image.ts and update the exports in src/lib/api/endpoints/index.ts to export * from "./clubs" and export * from "./image" so the filenames and import/exports follow the lowercase convention used by base, auth, bookstory, member, and book.src/lib/api/endpoints/Image.ts (1)
1-1: UnifyAPI_BASE_URLsource across endpoint modules.This file imports from
../endpointswhile sibling endpoint files use./base. Keeping one source avoids accidental base URL drift between modules.♻️ Suggested alignment
-import { API_BASE_URL } from "../endpoints"; +import { API_BASE_URL } from "./base";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/api/endpoints/Image.ts` at line 1, The import of API_BASE_URL in Image.ts currently comes from "../endpoints" which diverges from sibling modules; update the import to use the common source (import API_BASE_URL from "./base") so Image.ts uses the same base URL provider as other endpoint modules—locate the import statement at the top of Image.ts and replace the "../endpoints" module specifier with "./base" (keep the API_BASE_URL identifier and any existing named/default import shape intact).src/types/groups/clubsearch.ts (1)
1-1: Use a type-only import forApiResponse.Line 1 uses
ApiResponseonly in type aliases, soimport typeis safer and cleaner for TS module emit modes.Proposed fix
-import { ApiResponse } from "@/lib/api/types"; +import type { ApiResponse } from "@/lib/api/types";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/groups/clubsearch.ts` at line 1, The current import brings in ApiResponse as a value import even though it is only used in type aliases; change the import to a type-only import so TypeScript emits no value import (replace the existing import of ApiResponse with a type-only import), updating the import statement that references ApiResponse from "@/lib/api/types" and ensure all uses of ApiResponse remain as type annotations only (e.g., in any type aliases or interfaces).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/globals.css`:
- Around line 179-185: Move the WebKit scrollbar pseudo-element rule inside the
Tailwind utility definition so all no-scrollbar styles are together: update the
`@utility` no-scrollbar block (the existing block containing -ms-overflow-style
and scrollbar-width) to also include the .no-scrollbar::-webkit-scrollbar rule
instead of having it defined separately; ensure the pseudo-element selector
(.no-scrollbar::-webkit-scrollbar) is nested/placed within the same `@utility`
no-scrollbar declaration so the WebKit rule is applied wherever the utility is
used.
In `@src/app/groups/create/page.tsx`:
- Around line 294-315: The textarea for clubDescription (the JSX element using
value={clubDescription}, onChange calling setClubDescription and autoResize) is
missing an enforced character limit; add a maxLength={500} attribute to that
textarea and ensure the onChange handler (setClubDescription) also
guards/truncates input to 500 chars. Do the same for the other field referenced
in the comment (the shorter text input around the second diff, e.g., the
title/input with 40자 제한) by adding maxLength={40} and enforcing/truncating in
its onChange handler so the UI constraint is actually applied.
- Around line 122-136: The refetch result from nameQuery.refetch() must be
validated because TanStack Query v5 returns a result object rather than
throwing; after calling nameQuery.refetch() check r.isError and ensure r.data is
a boolean before using it: if r.isError or typeof r.data !== "boolean"
setNameCheck("idle") and toast.error("이름 중복 확인 실패"); only when r.data ===
true/false proceed to setNameCheck("duplicate"/"available") and call the
corresponding toast; alternatively pass { throwOnError: true } to
nameQuery.refetch() and keep the existing try/catch but prefer explicit
r.isError/type guard for clarity.
In `@src/app/groups/page.tsx`:
- Around line 226-227: Change the catch clause to accept unknown instead of any
and narrow the error before using its message: replace the catch parameter in
the try/catch around the group join flow with (e: unknown), then check (e
instanceof Error) to call toast.error(e.message) and fall back to
toast.error("가입 신청에 실패했습니다.") for non-Error values; this uses the existing
toast.error call and matches the error-handling pattern in PasswordEntry.tsx and
useProfileImage.ts.
- Around line 231-246: The onSubmitSearch handler calls refetchSearch
immediately after setAppliedParams which causes a stale manual refetch; remove
the refetchSearch() call from onSubmitSearch and let React Query auto-refetch
based on the changed appliedParams (i.e., update only inside onSubmitSearch:
call setAppliedParams({...}) and return), or if you truly need an immediate
fetch, compute the new params into a local const and call refetchSearch with
those explicit params instead of relying on state.
In `@src/components/base-ui/BookStory/bookstory_card_large.tsx`:
- Around line 56-64: Replace the non-semantic, clickable divs that handle
profile and like actions with semantic <button type="button"> elements so they
are keyboard-focusable and actionable (preserve visual classes by moving
className to the button), keep the existing handlers (onProfileClick,
onLikeClick) but move e.stopPropagation() into the button onClick and remove any
manual onKey handlers; also add an appropriate aria-label for screen readers
(e.g., "Open profile" / "Like story") and ensure buttons do not submit forms by
specifying type="button". This change applies to the block using onProfileClick
and the analogous like-action block (the second clickable div).
In `@src/components/base-ui/BookStory/bookstory_card.tsx`:
- Around line 137-146: Replace the clickable div used for the like control with
a semantic button element (same place where onClick stops propagation and calls
onLikeClick, referencing onLikeClick, likedByMe and likeCount) for both mobile
and desktop footers; keep the existing classes but move them to a <button
type="button">, preserve e.stopPropagation() in the onClick handler, add an
accessible label/aria-pressed (e.g., aria-pressed={likedByMe}) and ensure the
Image and span remain inside the button so keyboard users and screen readers can
interact correctly.
In `@src/components/base-ui/BookStory/bookstory_detail.tsx`:
- Around line 189-197: Replace the clickable divs used for the like control with
semantic button elements to enable keyboard access and ARIA state; in the mobile
block (the element wrapping Image heartIcon and the span showing 좋아요
{likeCount}) and the tablet/desktop block do the following: change the outer
<div> to a <button>, keep the onClick handler (onLikeClick) and
e.stopPropagation logic, add type="button", add aria-pressed={likedByMe} and an
accessible aria-label (e.g., `aria-label={likedByMe ? '좋아요 취소' : '좋아요'}`),
preserve classes (cursor, hover, p-1, rounded-full, transition-colors) and the
conditional text color using likedByMe; ensure the Image and span remain
children and remove any duplicate keyboard/role workarounds so both likeCount
and toggle behavior remain unchanged.
In `@src/components/base-ui/Group-Search/search_club_apply_modal.tsx`:
- Around line 67-75: The modal container div (the element that builds className
via ["w-full max-w-[1040px]", "flex flex-col items-end gap-4", ...].join(" "))
must expose dialog semantics: add role="dialog" and aria-modal="true", and set
aria-labelledby to the id of the modal title (create an id like
"apply-modal-title" on the title/header element). Also add an optional
aria-describedby pointing to the element with the modal body or instructions if
present, and ensure the title element has that matching id; apply the same
changes to the other modal container instance noted in the file (around the
second occurrence at 88).
In `@src/components/base-ui/Profile/OtherUser/ProfileUserInfo.tsx`:
- Around line 117-118: The ProfileUserInfo component is rendering hardcoded
zeros via the StatItem calls; update ProfileUserInfo to not render those
StatItem elements until real values are available by conditionally rendering
them (e.g. render <StatItem label="구독 중" count={subscriptionCount} /> and
<StatItem label="구독자" count={subscriberCount} /> only when
subscriptionCount/subscriberCount are non-null/defined or the API call has
returned), or hide the whole stats block during loading; tie the checks to the
actual data source used in this component (e.g. props/state like
subscriptionCount, subscriberCount or the fetchUserStats/useUser hook) so dummy
0s are never shown.
In `@src/hooks/mutations/useMemberMutations.ts`:
- Line 186: Replace the explicit any types used for the onError callback
parameters with unknown in src/hooks/mutations/useMemberMutations.ts — update
the onError signatures (the callbacks currently declared as onError: (error:
any, variables, context) => { ... }) to use onError: (error: unknown, variables,
context) => { ... } for all three occurrences (the handlers around lines ~55,
~117, and ~186), and if the handler body uses the error value, narrow or
stringify it (e.g., String(error) or check typeof) before passing to
console.error or other APIs to satisfy TypeScript safety.
- Around line 127-135: The throttling check currently inside mutationFn (using
followThrottleMap and now/lastTime) causes onMutate to run for throttled calls
and produce optimistic updates; move that throttle logic out so the mutation
never starts when blocked: remove the early-return throttle from mutationFn and
instead perform the 500ms check in the caller/wrapper that invokes the mutation
(e.g., the function that calls followMutation.mutate or the exported
toggleFollow handler), using followThrottleMap[nickname] to gate calling
followMutation.mutate so onMutate/onSettled never run for throttled clicks.
In `@src/hooks/mutations/useStoryMutations.ts`:
- Around line 149-160: Move the throttle logic out of mutationFn into onMutate:
in onMutate check likeThrottleMap[bookStoryId] and if throttled set
likeThrottleMap and return a context object {throttled: true} so you can skip
optimistic updates in onMutate and have onSettled check context.throttled to
skip cache invalidation; also keep a guard at the top of mutationFn (still
referencing likeThrottleMap and storyService.toggleLikeStory) to early-return if
throttled to avoid making the API call; finally, include storyKeys.all in the
invalidation list used by onSettled (alongside infiniteList, myList, list,
detail) so otherMember caches are refreshed.
- Around line 171-190: The otherMember caches (storyKeys.otherMember(nickname))
are not updated when toggling likes; update them optimistically in onMutate and
invalidate them in onSettled similar to the infinite/my lists. In onMutate, if
previousOtherMember (or however you store the previous cache for otherMember)
exists call
queryClient.setQueryData<InfiniteData<BookStoryListResponse>>(storyKeys.otherMember(nickname),
old => updateLikeInInfiniteList(old, bookStoryId)) or
queryClient.setQueryData<BookStoryListResponse>(storyKeys.otherMember(nickname),
old => updateLikeInStoryList(old, bookStoryId)) to match the shape; in onSettled
call queryClient.invalidateQueries(storyKeys.otherMember(nickname)) to refetch.
Ensure you reference storyKeys.otherMember, updateLikeInInfiniteList,
updateLikeInStoryList, queryClient.setQueryData and
queryClient.invalidateQueries when applying the change.
In `@src/lib/api/types.ts`:
- Around line 1-7: Remove the duplicate ApiResponse type declaration in this
file and instead rely on the canonical ApiResponse<T = unknown> defined in
src/types/auth.ts; either delete the local export of ApiResponse or replace it
with an import and re-export (e.g., import { ApiResponse } from 'src/types/auth'
and export type { ApiResponse }), ensuring no duplicate type declarations remain
and that usages across authService and other services reference the single
canonical type.
In `@src/services/clubService.ts`:
- Line 35: Replace the explicit any on the sanitization object by typing it
instead of using any: change "const cleaned: any = { ...params }" to a typed
version such as "const cleaned: Partial<typeof params> = { ...params }" or, if
there is a declared request param interface (e.g. ListClubsParams or
ClubQueryParams), use that like "const cleaned: Partial<ListClubsParams> = {
...params }"; ensure you import/use the actual params type where available so
the linter rule `@typescript-eslint/no-explicit-any` is satisfied and type safety
is preserved when cleaning request parameters.
In `@src/services/imageService.ts`:
- Around line 43-45: Check that the presigned response is successful and
contains a result before destructuring; update the code that uses
presigned.result (the variable presigned and its properties presigned.result,
presignedUrl, imageUrl) to first assert presigned?.isSuccess is true and
presigned?.result exists, throw or return a descriptive error if not, and only
then call putToPresignedUrl(presignedUrl, file, contentType); this ensures you
don't destructure undefined and surfaces a clear error when the presign API
fails.
In `@src/services/memberService.ts`:
- Around line 37-42: The getOtherProfile function currently returns
response.result! without guarding for failure; update getOtherProfile in
memberService.ts to check the ApiResponse (e.g. response.isSuccess and that
response.result is defined) after awaiting
apiClient.get(MEMBER_ENDPOINTS.GET_OTHER_PROFILE(nickname)) and throw a
descriptive error (or reject) when isSuccess is false or result is missing
instead of using the non-null assertion on response.result; use the ApiResponse
type and MEMBER_ENDPOINTS.GET_OTHER_PROFILE reference to locate the code.
---
Outside diff comments:
In `@src/app/`(main)/page.tsx:
- Around line 106-124: The BookStoryCardLarge instances are rendering the
current likedByMe state but not wiring the like action, so clicks do nothing;
add an onLikeClick prop to each BookStoryCardLarge that invokes the existing
like handler (e.g., call toggleLike or the component's like handler used
elsewhere) with the story identity and current liked state (for example pass
story.bookStoryId and story.likedByMe or an object the handler expects). Update
the mappings that create BookStoryCardLarge (the shown block and the other two
similar mappings) to pass onLikeClick={() => toggleLike({ bookStoryId:
story.bookStoryId, liked: story.likedByMe })} (or the equivalent signature your
toggleLike accepts) so likes are dispatched when the user taps the like button.
In `@src/components/base-ui/Group-Search/search_mybookclub.tsx`:
- Around line 75-85: Replace the clickable div used in displayGroups.map (the
element with key={group.id} and onClick={() =>
router.push(`/groups/${group.id}`)}) with Next.js Link navigation: import Link
from 'next/link', use <Link href={`/groups/${group.id}`} ...> to preserve the
existing className, children (the span showing group.name) and key, and remove
the useRouter import and router variable if they are no longer used elsewhere;
this restores native link behavior (keyboard, context menu, new-tab) and fixes
the accessibility issues from using a div with onClick.
---
Nitpick comments:
In `@src/lib/api/endpoints/Image.ts`:
- Line 1: The import of API_BASE_URL in Image.ts currently comes from
"../endpoints" which diverges from sibling modules; update the import to use the
common source (import API_BASE_URL from "./base") so Image.ts uses the same base
URL provider as other endpoint modules—locate the import statement at the top of
Image.ts and replace the "../endpoints" module specifier with "./base" (keep the
API_BASE_URL identifier and any existing named/default import shape intact).
In `@src/lib/api/endpoints/index.ts`:
- Around line 6-7: The exports in index.ts use PascalCase ("./Clubs", "./Image")
which is inconsistent with the existing lowercase module names; rename the
module files Clubs.ts → clubs.ts and Image.ts → image.ts and update the exports
in src/lib/api/endpoints/index.ts to export * from "./clubs" and export * from
"./image" so the filenames and import/exports follow the lowercase convention
used by base, auth, bookstory, member, and book.
In `@src/types/groups/clubsearch.ts`:
- Line 1: The current import brings in ApiResponse as a value import even though
it is only used in type aliases; change the import to a type-only import so
TypeScript emits no value import (replace the existing import of ApiResponse
with a type-only import), updating the import statement that references
ApiResponse from "@/lib/api/types" and ensure all uses of ApiResponse remain as
type annotations only (e.g., in any type aliases or interfaces).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
next.config.tssrc/app/(main)/books/[id]/page.tsxsrc/app/(main)/page.tsxsrc/app/(main)/profile/[nickname]/page.tsxsrc/app/(main)/stories/[id]/page.tsxsrc/app/(main)/stories/page.tsxsrc/app/globals.csssrc/app/groups/create/page.tsxsrc/app/groups/groupSearchDummy.tssrc/app/groups/page.tsxsrc/components/base-ui/BookStory/bookstory_card.tsxsrc/components/base-ui/BookStory/bookstory_card_large.tsxsrc/components/base-ui/BookStory/bookstory_detail.tsxsrc/components/base-ui/Group-Search/search_club_apply_modal.tsxsrc/components/base-ui/Group-Search/search_clublist/search_clublist_item.tsxsrc/components/base-ui/Group-Search/search_mybookclub.tsxsrc/components/base-ui/MyPage/MyBookStoryList.tsxsrc/components/base-ui/Profile/BookStoryList.tsxsrc/components/base-ui/Profile/OtherUser/OtherUserProfileTabs.tsxsrc/components/base-ui/Profile/OtherUser/ProfileUserInfo.tsxsrc/components/base-ui/home/home_bookclub.tsxsrc/components/base-ui/home/list_subscribe_element.tsxsrc/components/base-ui/home/list_subscribe_large.tsxsrc/hooks/mutations/useCreateClubMutation.tssrc/hooks/mutations/useMemberMutations.tssrc/hooks/mutations/useSearchClubMutations.tssrc/hooks/mutations/useStoryMutations.tssrc/hooks/queries/useCreateClubQueries.tssrc/hooks/queries/useMemberQueries.tssrc/hooks/queries/useSearchClubQueries.tssrc/hooks/queries/useStoryQueries.tssrc/lib/api/endpoints/Clubs.tssrc/lib/api/endpoints/Image.tssrc/lib/api/endpoints/bookstory.tssrc/lib/api/endpoints/index.tssrc/lib/api/endpoints/member.tssrc/lib/api/types.tssrc/services/clubService.tssrc/services/imageService.tssrc/services/memberService.tssrc/services/storyService.tssrc/types/groups/clubCreate.tssrc/types/groups/clubsearch.tssrc/types/member.tssrc/utils/groupMapper.ts
💤 Files with no reviewable changes (1)
- src/app/groups/groupSearchDummy.ts
| try { | ||
| const r = await nameQuery.refetch(); | ||
| const isDuplicate = r.data; // boolean | ||
|
|
||
| if (isDuplicate) { | ||
| setNameCheck("duplicate"); | ||
| toast.error("이미 존재하는 모임 이름입니다."); | ||
| } else { | ||
| setNameCheck("available"); | ||
| toast.success("사용 가능한 모임 이름입니다."); | ||
| } | ||
| } catch { | ||
| setNameCheck("idle"); | ||
| toast.error("이름 중복 확인 실패"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
TanStack React Query v5 refetch behavior error handling
💡 Result:
Refetch behavior in TanStack React Query v5 (and how errors are handled)
When refetching happens
A query can refetch automatically when it’s stale and one of these events happens (defaults depend on option):
- Window/tab becomes visible again (
refetchOnWindowFocus, defaulttrue) [1] - Reconnect / remount (controlled by
refetchOnReconnect,refetchOnMount) - Intervals (
refetchInterval) [2]
If you don’t want those background refetches, disable them per-query (or globally), e.g. refetchOnWindowFocus: false [1].
What “error state” means (and retries)
- If your
queryFnthrows, the query goes tostatus: 'error'/isError: trueand exposes the error onerror[3]. - By default, Query will retry failed queries (default retry count is 3). During retries, the error is exposed via
failureReason, and only becomeserrorafter the final failed attempt [2].
Manual refetch() and errors (important in v5)
- Calling
refetch()does not throw by default when the refetch fails; it “only logs”. - If you want
refetch()to throw, call it withthrowOnError: true(also supportscancelRefetch) [4].
await refetch({ throwOnError: true })queryClient.refetchQueries() / invalidateQueries() and throwing on refetch failures
Similarly, queryClient.refetchQueries() (and invalidateQueries which uses it for refetching) will not throw by default if some refetches fail—but you can opt into throwing with throwOnError: true [5].
v5 change: onError / onSuccess / onSettled removed from queries
In v5, query callbacks like onError, onSuccess, onSettled were removed from useQuery / QueryObserver (mutations still have them). Handle side-effects in React (e.g. useEffect) or other patterns instead [6].
Error boundaries in v5
useErrorBoundarywas renamed tothrowOnError(foruseQuery) [6].- If you use error boundaries, use
useQueryErrorResetBoundary()to reset query errors when the boundary resets [7].
Sources
[1] TanStack docs: Window Focus Refetching (v5) (tanstack.com)
[2] TanStack docs: Query Retries (v5) (tanstack.com)
[3] TanStack docs: Queries guide (states: pending/error/success, error, isFetching) (tanstack.com)
[4] TanStack docs: useQuery reference (refetch({ throwOnError })) (zh-hant.tanstack.dev)
[5] TanStack docs: QueryClient (refetchQueries / invalidateQueries + throwOnError) (tanstack.com)
[6] TanStack docs: Migrating to v5 (removed query callbacks; useErrorBoundary -> throwOnError) (tanstack.com)
[7] TanStack docs: useQueryErrorResetBoundary (tanstack.com)
Citations:
- 1: https://tanstack.com/query/v5/docs/framework/react/guides/window-focus-refetching?utm_source=openai
- 2: https://tanstack.com/query/v5/docs/react/guides/query-retries?utm_source=openai
- 3: https://tanstack.com/query/v5/docs/framework/react/guides/queries?utm_source=openai
- 4: https://zh-hant.tanstack.dev/query/v5/docs/framework/react/reference/useQuery?utm_source=openai
- 5: https://tanstack.com/query/v5/docs/reference/QueryClient?utm_source=openai
- 6: https://tanstack.com/query/v5/docs/react/guides/migrating-to-v5?utm_source=openai
- 7: https://tanstack.com/query/v5/docs/react/reference/useQueryErrorResetBoundary?utm_source=openai
🏁 Script executed:
# Examine the file and context around the flagged lines
cat -n src/app/groups/create/page.tsx | sed -n '115,145p'Repository: checkmo2025/FE
Length of output: 1051
🏁 Script executed:
# Check package.json for TanStack Query version
cat package.json | grep -A 5 -B 5 "react-query\|@tanstack"Repository: checkmo2025/FE
Length of output: 423
🏁 Script executed:
# Search for nameQuery definition in the file
rg "nameQuery" src/app/groups/create/page.tsx -B 5 -A 2Repository: checkmo2025/FE
Length of output: 506
Handle failed name refetch explicitly—don't let undefined r.data silently pass as "available".
In TanStack Query v5, refetch() does not throw on failure by default; it returns a result object with isError: true. Since the code calls refetch() without throwOnError: true, the catch block never executes on network/API failures. This means failed name checks fall through to the "else" branch, incorrectly marking duplicate checks as "available".
Add an explicit guard after refetch to check both r.isError and the type of r.data before proceeding:
🛠️ Suggested fix
try {
const r = await nameQuery.refetch();
+ if (r.isError || typeof r.data !== "boolean") {
+ setNameCheck("idle");
+ toast.error("이름 중복 확인 실패");
+ return;
+ }
const isDuplicate = r.data;
if (isDuplicate) {
setNameCheck("duplicate");
toast.error("이미 존재하는 모임 이름입니다.");
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| const r = await nameQuery.refetch(); | |
| const isDuplicate = r.data; // boolean | |
| if (isDuplicate) { | |
| setNameCheck("duplicate"); | |
| toast.error("이미 존재하는 모임 이름입니다."); | |
| } else { | |
| setNameCheck("available"); | |
| toast.success("사용 가능한 모임 이름입니다."); | |
| } | |
| } catch { | |
| setNameCheck("idle"); | |
| toast.error("이름 중복 확인 실패"); | |
| } | |
| try { | |
| const r = await nameQuery.refetch(); | |
| if (r.isError || typeof r.data !== "boolean") { | |
| setNameCheck("idle"); | |
| toast.error("이름 중복 확인 실패"); | |
| return; | |
| } | |
| const isDuplicate = r.data; // boolean | |
| if (isDuplicate) { | |
| setNameCheck("duplicate"); | |
| toast.error("이미 존재하는 모임 이름입니다."); | |
| } else { | |
| setNameCheck("available"); | |
| toast.success("사용 가능한 모임 이름입니다."); | |
| } | |
| } catch { | |
| setNameCheck("idle"); | |
| toast.error("이름 중복 확인 실패"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/groups/create/page.tsx` around lines 122 - 136, The refetch result
from nameQuery.refetch() must be validated because TanStack Query v5 returns a
result object rather than throwing; after calling nameQuery.refetch() check
r.isError and ensure r.data is a boolean before using it: if r.isError or typeof
r.data !== "boolean" setNameCheck("idle") and toast.error("이름 중복 확인 실패"); only
when r.data === true/false proceed to setNameCheck("duplicate"/"available") and
call the corresponding toast; alternatively pass { throwOnError: true } to
nameQuery.refetch() and keep the existing try/catch but prefer explicit
r.isError/type guard for clarity.
There was a problem hiding this comment.
main 브랜치로 PR을 올려서 잘못 나온 Review 입니다.
There was a problem hiding this comment.
@shinwokkang, 이해했습니다. PR이 main 브랜치로 잘못 올라가서 이 리뷰가 적용되지 않는다는 것을 확인했습니다. 올바른 브랜치로 PR을 다시 올리시면 다시 리뷰하겠습니다.
| // ~/types/auth.ts에 정의되어 있는건 아는데 거기 있는 거보단 여기가 맞을 거 같아서 빼두겠습니다. | ||
| export type ApiResponse<T> = { | ||
| isSuccess: boolean; | ||
| code: string; | ||
| message: string; | ||
| result: T; | ||
| }; No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if `@/lib/api/types` is imported anywhere
echo "=== Searching for imports from `@/lib/api/types` ==="
rg "@/lib/api/types" --type=ts --type=tsx
# Check for ApiResponse definition in auth.ts
echo -e "\n=== Searching for ApiResponse in auth.ts files ==="
rg -n "ApiResponse" --type=ts -g '*auth*'
# Check all ApiResponse definitions across the codebase
echo -e "\n=== All ApiResponse definitions ==="
rg "export.*type.*ApiResponse|export.*interface.*ApiResponse" --type=ts -A2
# Check where ApiResponse is used
echo -e "\n=== ApiResponse usage across codebase ==="
rg "ApiResponse" --type=ts --type=tsx -c | head -20Repository: checkmo2025/FE
Length of output: 3731
Remove the duplicate ApiResponse type definition from this file.
The new ApiResponse type in src/lib/api/types.ts is not imported anywhere in the codebase. The actual definition being used is ApiResponse<T = unknown> in src/types/auth.ts (line 15), which is actively used throughout authService.ts and other services. This file contains unused duplicate code; either delete it or clarify its intended purpose.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/api/types.ts` around lines 1 - 7, Remove the duplicate ApiResponse
type declaration in this file and instead rely on the canonical ApiResponse<T =
unknown> defined in src/types/auth.ts; either delete the local export of
ApiResponse or replace it with an import and re-export (e.g., import {
ApiResponse } from 'src/types/auth' and export type { ApiResponse }), ensuring
no duplicate type declarations remain and that usages across authService and
other services reference the single canonical type.
| function safeImageSrc(src?: string | null) { | ||
| if (!src) return DEFAULT_CLUB_IMG; | ||
|
|
||
| if (src === "string") return DEFAULT_CLUB_IMG; |
There was a problem hiding this comment.
src === "string" 조건은 src가 string | null | undefined 타입이므로 항상 false입니다. !src가 null, undefined, ""를 모두 처리하므로 이 줄은 제거하거나 typeof src !== "string"으로 변경하는 것이 더 정확합니다. 현재는 !src 조건으로 인해 기능상 문제는 없지만, 혼란을 줄 수 있습니다.
| if (src === "string") return DEFAULT_CLUB_IMG; | |
| if (src.startsWith("/")) return src; |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/hooks/mutations/useStoryMutations.ts (1)
155-167:⚠️ Potential issue | 🟠 MajorThrottle check is placed too late, so throttled clicks still run optimistic + invalidation flow.
At Line 155, the throttle is in
mutationFn, butonMutate(Line 167) runs first in React Query. Throttled clicks still do optimistic updates and then trigger Line 247 invalidation, causing flicker/refetch without an API call.Suggested fix
mutationFn: async (bookStoryId: number) => { const now = Date.now(); const lastTime = likeThrottleMap[bookStoryId] || 0; // Throttle: 500ms if (now - lastTime < 500) { return; } likeThrottleMap[bookStoryId] = now; return storyService.toggleLikeStory(bookStoryId); }, onMutate: async (bookStoryId) => { + const now = Date.now(); + const lastTime = likeThrottleMap[bookStoryId] || 0; + if (now - lastTime < 500) { + return { throttled: true as const }; + } + // Cancel any outgoing refetches await queryClient.cancelQueries({ queryKey: storyKeys.all }); // Snapshot the previous values const previousInfiniteStories = queryClient.getQueryData(storyKeys.infiniteList()); @@ return { + throttled: false as const, previousInfiniteStories, previousMyStories, previousStories, previousStoryDetail, previousOtherMemberStories, }; }, onError: (err, bookStoryId, context) => { + if (context?.throttled) return; console.error("Failed to toggle like:", err); toast.error("좋아요 상태 업데이트에 실패했습니다."); @@ - onSettled: (data, err, bookStoryId) => { + onSettled: (data, err, bookStoryId, context) => { + if (context?.throttled) return; // Invalidate queries to ensure sync with server queryClient.invalidateQueries({ queryKey: storyKeys.infiniteList() }); queryClient.invalidateQueries({ queryKey: storyKeys.myList() }); queryClient.invalidateQueries({ queryKey: [...storyKeys.all, "otherMember"] }); queryClient.invalidateQueries({ queryKey: storyKeys.list() }); queryClient.invalidateQueries({ queryKey: storyKeys.detail(bookStoryId) }); },#!/bin/bash # Verify throttle guard placement and throttled-context skips in mutation lifecycle. rg -n "mutationFn: async|onMutate: async|if \\(now - lastTime < 500\\)|throttled|onError:|onSettled:" src/hooks/mutations/useStoryMutations.ts -C 2Expected after fix:
- Throttle check exists at top of
onMutate.onErrorandonSettledboth short-circuit oncontext?.throttled.Also applies to: 167-170, 247-253
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/mutations/useStoryMutations.ts` around lines 155 - 167, Move the 500ms throttle check out of mutationFn and into the very first lines of onMutate so clicks are short-circuited before any optimistic update; use the existing likeThrottleMap and bookStoryId to compute now/lastTime and, if throttled, return a context object like { throttled: true } so the mutation lifecycle knows to skip further work. In addition, update onError and onSettled to guard on context?.throttled and immediately return (no rollback or invalidation) when true. Keep mutationFn calling storyService.toggleLikeStory(bookStoryId) only for non-throttled cases. Ensure the same pattern is applied for handlers referencing onError/onSettled to prevent flicker/refetch.
🧹 Nitpick comments (1)
src/hooks/mutations/useStoryMutations.ts (1)
54-55: Defensively clampcommentCountto zero when applying deltas.
commentCount + deltacan show negative values in stale-cache edge cases (especially delete path). Clamping avoids invalid UI states.Suggested fix
- commentCount: story.commentCount + delta, + commentCount: Math.max(0, story.commentCount + delta),- commentCount: story.commentCount + delta, + commentCount: Math.max(0, story.commentCount + delta),Also applies to: 72-73
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/mutations/useStoryMutations.ts` around lines 54 - 55, The update that sets commentCount can go negative in stale-cache edge cases; change the assignment in useStoryMutations that currently does "commentCount: story.commentCount + delta" to clamp at zero (e.g., use Math.max(0, story.commentCount + delta)) so commentCount never becomes negative, and apply the same change to the second occurrence around lines 72-73.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/hooks/mutations/useStoryMutations.ts`:
- Around line 155-167: Move the 500ms throttle check out of mutationFn and into
the very first lines of onMutate so clicks are short-circuited before any
optimistic update; use the existing likeThrottleMap and bookStoryId to compute
now/lastTime and, if throttled, return a context object like { throttled: true }
so the mutation lifecycle knows to skip further work. In addition, update
onError and onSettled to guard on context?.throttled and immediately return (no
rollback or invalidation) when true. Keep mutationFn calling
storyService.toggleLikeStory(bookStoryId) only for non-throttled cases. Ensure
the same pattern is applied for handlers referencing onError/onSettled to
prevent flicker/refetch.
---
Nitpick comments:
In `@src/hooks/mutations/useStoryMutations.ts`:
- Around line 54-55: The update that sets commentCount can go negative in
stale-cache edge cases; change the assignment in useStoryMutations that
currently does "commentCount: story.commentCount + delta" to clamp at zero
(e.g., use Math.max(0, story.commentCount + delta)) so commentCount never
becomes negative, and apply the same change to the second occurrence around
lines 72-73.
📌 개요 (Summary)
🛠️ 변경 사항 (Changes)
📸 스크린샷 (Screenshots)
(UI 변경 사항이 있다면 첨부해주세요)
✅ 체크리스트 (Checklist)
pnpm build)pnpm lint)Summary by CodeRabbit
New Features
Improvements